Repository navigation
Add preflight LLM check before dispatching evaluations - #2109
Conversation
1d611a7 to
ace16d9
Compare
all-hands-bot
left a comment
There was a problem hiding this comment.
🟢 Good taste - Simple, pragmatic solution to a real problem
Solid implementation. You have a documented failure (Qwen3-Coder-480B wasting compute on unavailable model), and you're adding a simple preflight check that makes a test call before dispatching expensive eval jobs.
What's right:
- Focused ~150 line script solving one problem
- Clear error messages with visual indicators (✓/✗)
- Proper timeout and error handling
- Fails fast to save resources
- No over-engineering
Minor nit: See inline comment about dependency pinning.
VERDICT: ✅ Approved - Solves real problem with appropriate complexity.
8a30b5c to
fdabf79
Compare
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Acceptable - Solid pragmatic solution, but missing test coverage for new logic.
all-hands-bot
left a comment
There was a problem hiding this comment.
🟢 Good taste - Simple, pragmatic solution to a real problem
You have a documented production failure (Qwen3-Coder-480B wasting compute on unavailable model), and you're adding a simple preflight check that makes a test call before dispatching expensive eval jobs.
What's right:
- Focused ~150 line change solving one problem
- Clean exception handling with specific error types → catch-all
- Proper escape hatches (SKIP_PREFLIGHT, skip when no API key)
- Fails fast with clear error messages (✓/✗ indicators)
- Comprehensive test coverage added after feedback
- No over-engineering
Key insight: The best solutions are boring. This checks if the model works before spending money. Done.
LGTM! 🚀
|
@OpenHands fix pre-commit, merge conflicts, and re-run the failed review thread gate github CI |
|
I'm on it! neubig can track my progress at all-hands.dev |
Integrated the preflight check directly into resolve_model_config.py using litellm. The script now: 1. Resolves model IDs to configurations 2. Makes a test call to each model via litellm with all config parameters 3. Fails fast if any model is unreachable The check is optional and gracefully skips if: - LLM_API_KEY is not set - litellm is not installed - SKIP_PREFLIGHT=true is set This would have caught the Qwen3-Coder-480B issue where all 16 commit0 instances failed because the Fireworks AI endpoint was not responding. Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
- Remove conditional HAS_LITELLM check (litellm is installed in workflow) - Add tests for test_model() covering success, errors, and config params - Add tests for run_preflight_check() covering skip conditions and pass/fail Co-authored-by: openhands <openhands@all-hands.dev>
21472f8 to
25519b4
Compare
Summary of ChangesI have completed all three requested tasks for PR #2109: ✅ 1. Fixed Merge Conflicts
✅ 2. Fixed Pre-commit IssuesRuff auto-fixed two minor issues in
✅ 3. Re-ran Failed Review Thread Gate CI
Changes PushedThe rebased and fixed branch has been force-pushed to
The PR is now mergeable with no conflicts. |
Co-authored-by: openhands <openhands@all-hands.dev>
Summary
This adds a simple verification step that makes a test call to each model before dispatching the full evaluation workflow. This prevents wasting compute resources when a model is unavailable or returning errors.
Problem
The Qwen3-Coder-480B commit0 evaluation failed with:
"Instance failed after 3 retries. Last error: Conversation run failed for id=...: Remote conversation ended with error"The
output.jsonlwas empty because the LLM API (Fireworks AI) was not responding. The evaluation infrastructure retried 4 times per instance with increasing resources but failed each time because the underlying issue was model availability, not infrastructure.Solution
Add a preflight check that:
This runs before dispatching the evaluation to Kubernetes, saving compute resources and providing clearer error messages.
Example output
Success:
Failure:
@neubig can click here to continue refining the PR
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:64ba82e-pythonRun
All tags pushed for this build
About Multi-Architecture Support
64ba82e-python) is a multi-arch manifest supporting both amd64 and arm6464ba82e-python-amd64) are also available if needed